Skip to content

Conversation

@jaul-nsc
Copy link
Contributor

Align the nRF_Connect_SDK_13_14.py THCI file with the GRL Thread's certification framework in version 64.0

Copilot AI review requested due to automatic review settings November 27, 2025 09:50
@jaul-nsc jaul-nsc requested review from a team as code owners November 27, 2025 09:50
@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 27, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the nRF_Connect_SDK_13_14.py THCI (Thread Host Controller Interface) file with GRL Thread's certification framework version 64.0. The changes primarily involve updating API methods, fixing parameter ordering, and adding new functionality to support the latest certification requirements.

Key changes:

  • Added new API methods for ICMP ping, SRP service registration, TCP HTTP requests, and NAT64 prefix handling
  • Fixed parameter ordering in SRP-related methods (swapping priority and weight parameters)
  • Enhanced UDP and DNS query methods with additional options and parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -3736,7 +3765,9 @@ def get_own_omr_address(self, omr_prefix):

def srp_client_remove(self, instancename, servicename):
cmd = "srp client service remove %s %s" % (instancename, servicename)
# cmd = 'srp client service remove service-test-1 _thread-test._udp'
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code. If this is an example, consider adding it to documentation instead.

Suggested change
# cmd = 'srp client service remove service-test-1 _thread-test._udp'

Copilot uses AI. Check for mistakes.
self.__executeCommand(cmd)
# self.__executeCommand("netdata register")
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code. If this command should be executed conditionally, implement proper conditional logic instead.

Suggested change
# self.__executeCommand("netdata register")

Copilot uses AI. Check for mistakes.
self.__executeCommand(cmd)
if ip_addr:
cmd += f" {ip_addr} 53 6000 3 1"
print(cmd)
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace print statement with proper logging using self.log() as used elsewhere in the file.

Copilot uses AI. Check for mistakes.
tcp_request = self.__executeCommand(
"tcp send -x %s" % hexa_http_request, ignore_exec=True
)
print(tcp_request)
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace print statement with proper logging using self.log() as used elsewhere in the file.

Suggested change
print(tcp_request)
self.log(tcp_request)

Copilot uses AI. Check for mistakes.
time.sleep(3)
self.__executeCommand("tcp sendend", ignore_exec=True)
self.__executeCommand("tcp deinit", ignore_exec=True)
return "Failed" if tcp_request is True else True
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is inverted and confusing. When tcp_request is True (success with ignore_exec), it returns 'Failed' string. When it's not True, it returns True boolean. This should return consistent boolean values: return tcp_request is not True.

Suggested change
return "Failed" if tcp_request is True else True
return tcp_request is not True

Copilot uses AI. Check for mistakes.
Comment on lines +4362 to +4363
else:
continue
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else-continue is unnecessary. Remove the else clause since continue is only needed when the condition is false.

Suggested change
else:
continue
continue

Copilot uses AI. Check for mistakes.
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 27, 2025

CI Information

To view the history of this post, click the 'edited' button above
Build number: 5

Inputs:

Sources:

sdk-nrf: PR head: 9fa68304ce5e3b36879db9770b43adfb3f36757a

more details

sdk-nrf:

PR head: 9fa68304ce5e3b36879db9770b43adfb3f36757a
merge base: 78f8c43b230ef210d5882b222ee8e0b5960cb904
target head (main): 0c79406a493836eff01af3bf4a75b49a5a47ca16
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (1)
samples
│  ├── openthread
│  │  ├── cli
│  │  │  ├── harness-thci
│  │  │  │  │ nRF_Connect_SDK_13_14.py

Outputs:

Toolchain

Version: 964ddb2c70
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:964ddb2c70_5ea73affbf

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 53
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-thread-main
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

Copy link
Contributor

@ArekBalysNordic ArekBalysNordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ADNO

@jaul-nsc jaul-nsc added this to the 3.2.0 milestone Nov 27, 2025
@jaul-nsc jaul-nsc force-pushed the feature/align-thci-to-grl-v64 branch 3 times, most recently from e465849 to b9225ae Compare November 27, 2025 10:47
Align the nRF_Connect_SDK_13_14.py THCI file
with the GRL Thread's certification framework
in version 64.0.

Signed-off-by: Jakub Uliarczyk <[email protected]>
Copilot AI review requested due to automatic review settings November 27, 2025 13:35
@jaul-nsc jaul-nsc force-pushed the feature/align-thci-to-grl-v64 branch from b9225ae to 9fa6830 Compare November 27, 2025 13:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants